Skip to content

Comments

feat: use event producer config#32874

Closed
navinkarkera wants to merge 1 commit intoopenedx:masterfrom
open-craft:navin/use-producer-config
Closed

feat: use event producer config#32874
navinkarkera wants to merge 1 commit intoopenedx:masterfrom
open-craft:navin/use-producer-config

Conversation

@navinkarkera
Copy link
Contributor

@navinkarkera navinkarkera commented Jul 28, 2023 •
edited
Loading

Description

Replace openedx_event signal handlers with new producer config to push to event bus.

Supporting information

Testing instructions

  • Setup master devstack.
  • Get lms and cms up using make {lms,cms}-up
  • Go to cms shell using make cms-shell
  • Run pip install -r requirements/edx/development.txt to install new openedx_events version.
  • Exit from cms-shell and see cms logs using make cms-logs.
  • Go to studio and delete an xblock or publish a section/subsection.
  • This should push the corresponding event to event bus and print below message: 
    edx.devstack.cms  | 2023-07-28 13:43:57,201 INFO 515 [edx_event_bus_redis.internal.producer] [user 3] [ip 172.18.0.1] producer.py:82 - Message delivered to Redis event bus: topic=dev-content-authoring-xblock-lifecycle, message_id=cccfef82-2d4c-11ee-9c7a-0242ac12000c, signal=<OpenEdxPublicSignal: org.openedx.content_authoring.xblock.published.v1>, redis_msg_id=b'1690551837201-0'
  • Play with the EVENT_BUS_PRODUCER_CONFIG by toggling the flag and triggering the event.

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

@navinkarkera navinkarkera marked this pull request as ready for review July 28, 2023 13:56
@navinkarkera
feat: use event producer config
b46d149 
Replace openedx_event signal handlers with new producer config to push to event bus
@navinkarkera navinkarkera force-pushed the navin/use-producer-config branch from cc6b931 to b46d149 Compare July 28, 2023 13:59
navinkarkera
navinkarkera commented Jul 28, 2023
View reviewed changes
requirements/edx/doc.txt
openedx-django-wiki==2.0.0
# via -r requirements/edx/base.txt
openedx-events==8.3.0
openedx-events @ git+https://github.com/open-craft/openedx-events@navin/configurable-handler
Copy link
Contributor Author

@navinkarkera navinkarkera Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: update this requirement once openedx/openedx-events#249 is merged

@navinkarkera navinkarkera mentioned this pull request Jul 28, 2023
Closed
10 tasks
tecoholic
tecoholic approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@tecoholic tecoholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@navinkarkera 👍 LGTM. Good to see some code go away :)

  • I tested this: Followed the testing instructions and verified that the events are getting published as per the configuration
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation - inline docs present

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 3, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Aug 3, 2023 •
edited
Loading

Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@rgraber rgraber reopened this Aug 31, 2023
@rgraber rgraber mentioned this pull request Aug 31, 2023
Closed
3 tasks
@mphilbrick211
Copy link

mphilbrick211 commented Sep 12, 2023

Hi @navinkarkera! Flagging the new tests per the recent update: https://discuss.openedx.org/t/minor-change-to-edx-platform-check-statuses/11131

@rgraber
Copy link
Contributor

rgraber commented Sep 14, 2023 •
edited
Loading

@navinkarkera how would we feel about leaving events disabled by default until we figure out openedx/openedx-events#265 ?
I can take on that work if you're ok with it.

@navinkarkera
Copy link
Contributor Author

navinkarkera commented Sep 15, 2023

I can take on that work if you're ok with it.

@rgraber Sure, feel free to create a new PR with the commits or however you see fit.

@rgraber
Copy link
Contributor

rgraber commented Sep 15, 2023

I got git into a weird state and ended up just opening another PR here: #33269 . The only things that changed were openedx-events was set to 8.6.0 rather than a branch and all events start disabled in common.py

@rgraber rgraber closed this Sep 15, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Sep 15, 2023

@navinkarkera Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

1 similar comment
@openedx-webhooks
Copy link

openedx-webhooks commented Sep 15, 2023

@navinkarkera Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@rgraber rgraber mentioned this pull request Sep 15, 2023
Closed
@rgraber rgraber mentioned this pull request Oct 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tecoholic tecoholic tecoholic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Contributions
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@navinkarkera @openedx-webhooks @mphilbrick211 @rgraber @tecoholic